rpc: fix MatchMirror substring ambiguity for exact-match names - #227
Conversation
When a mirror name is a substring of other mirror names (e.g. "fcix.net" vs. "mirror.fcix.net" and "paducahix.mm.fcix.net"), MatchMirror used a plain strings.Contains scan, so every mirror whose name contained the pattern would show up as a match. This made it impossible to address the shorter mirror on its own via "mb edit" or similar commands, since the CLI always reported "Multiple match" and aborted. Extract the matching logic into matchMirrorsByPattern and prefer a single case-insensitive exact match over partial substring matches, while keeping the existing substring-match behavior when no exact match exists. Fixes videolabs#134 This PR was written primarily by Claude Code; I reviewed the change and ran the tests before submitting.
|
Is this AI generated ? |
Yes the changes are AI assisted and I have reviewed the changes. |
|
Can you ask Claude to refactor the tests in order to use sub-tests? All the tests can be defined by 3 variables:
Then the logic to run and check the results is the same for all the tests. So it should be trivial to use sub-tests and make it all much more concise, only one main test function |
- Convert 5 separate test functions for matchMirrorsByPattern into a single table-driven test using t.Run, as requested by reviewer. - Fix copyright header on rpc_test.go to reflect the actual author. - Correct regression test URL from etix/mirrorbits to videolabs/mirrorbits. - Adopt improved function comment wording suggested by reviewer.
|
Pushed a follow-up commit addressing all review feedback:
🤖 Addressed by Claude Code |
|
LGTM now. Thanks! @jbkempf If you want to merge this MR, I suggest to squash commits and keep only the commit message from the first commit (I don't know if GitHub allows you do that easily though). |
Done ✔️ |
3 participants
Problem
mb edit <name>(and other commands that resolve a mirror by name)matches mirrors using a plain
strings.Containsscan over all mirrornames. When a mirror's name happens to be a substring of one or more
other mirror names, it can never be addressed on its own: every mirror
whose name contains the pattern is reported as a match, so the CLI
always prints "Multiple match" and exits.
For example, given mirrors
fcix.net,mirror.fcix.net, andpaducahix.mm.fcix.net, runningmb edit fcix.netreports all threeas matches instead of resolving to the exact
fcix.netmirror.Fixes #134
Fix
MatchMirror's matching logic is extracted into a standalone,testable function
matchMirrorsByPattern. It now prefers a singlecase-insensitive exact match over partial substring matches. If no
exact match exists, it falls back to the previous substring-match
behavior (including returning multiple matches when the pattern is
genuinely ambiguous).
Testing
rpc/rpc_test.gocovering: the exact-match-among-substringsregression case from Cannot edit a mirror when its ID is a substring of another #134, case-insensitivity of the exact match,
preserved multi-match behavior when no exact match exists, no-match,
and a basic single-substring-match case.
strings.Contains-onlylogic and passes with the fix.
go build/go vetclean forGOOS=linux(this repo's CI target);the local Windows dev environment can't compile the
processpackageor the original
rpc/rpc.go(syscall.SIGUSR2is Unix-only), which isa pre-existing platform limitation unrelated to this change.
This PR was written primarily by Claude Code; I reviewed the change and
ran the tests before submitting.